-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
better errors for standalone explicit generic instantiations #24276
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
metagn
changed the title
better errors for explicit generic instantiations
better errors for standalone explicit generic instantiations
Oct 10, 2024
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
Araq
pushed a commit
that referenced
this pull request
Nov 6, 2024
fixes issue described in https://forum.nim-lang.org/t/12579 In #24065 explicit generic parameter matching was made to fail matches on arguments with unresolved types in generic contexts (the sigmatch diff, following #24010), similar to what is done for regular calls since #22029. However unlike regular calls, a failed match in a generic context for a standalone explicit generic instantiation did not convert the expression into one with `tyFromExpr` type, which means it would error immediately given any unresolved parameter. This is now done to fix the issue. For explicit generic instantiations on single non-overloaded symbols, a successful match is still instantiated. For multiple overloads (i.e. symchoice), if any of the overloads fail the match, the entire expression is considered untyped and any instantiations are not used, so as to not void overloads that would match later. This means even symchoices without unresolved arguments aren't instantiated, which may be too restrictive, but it could also be too lenient and we might need to make symchoice instantiations always untyped. The behavior for symchoice is not sound anyway given it causes #9997 so this is something to consider for a redesign. Diff follows #24276.
narimiran
pushed a commit
that referenced
this pull request
Jan 14, 2025
refs #8064, refs #24010 Error messages for standalone explicit generic instantiations are revamped. Failing standalone explicit generic instantiations now only error after overloading has finished and resolved to the default `[]` magic (this means `[]` can now be overloaded for procs but this isn't necessarily intentional, in #24010 it was documented that it isn't possible). The error messages for failed instantiations are also no longer a simple `cannot instantiate: foo` message, instead they now give the same type mismatch error message as overloads with mismatching explicit generic parameters. This is now possible due to the changes in #24010 that delegate all explicit generic proc instantiations to overload resolution. Old code that worked around this is now removed. `maybeInstantiateGeneric` could maybe also be removed in favor of just `explicitGenericSym`, the `result == n` case is due to `explicitGenericInstError` which is only for niche cases. Also, to cause "ambiguous identifier" error messages when the explicit instantiation is a symchoice and the expression context doesn't allow symchoices, we semcheck the sym/symchoice created by `explicitGenericSym` with the given expression flags. #8064 isn't entirely fixed because the error message is still misleading for the original case which does `values[1]`, as a consequence of #12664. (cherry picked from commit 0a058a6)
narimiran
pushed a commit
that referenced
this pull request
Jan 14, 2025
fixes issue described in https://forum.nim-lang.org/t/12579 In #24065 explicit generic parameter matching was made to fail matches on arguments with unresolved types in generic contexts (the sigmatch diff, following #24010), similar to what is done for regular calls since #22029. However unlike regular calls, a failed match in a generic context for a standalone explicit generic instantiation did not convert the expression into one with `tyFromExpr` type, which means it would error immediately given any unresolved parameter. This is now done to fix the issue. For explicit generic instantiations on single non-overloaded symbols, a successful match is still instantiated. For multiple overloads (i.e. symchoice), if any of the overloads fail the match, the entire expression is considered untyped and any instantiations are not used, so as to not void overloads that would match later. This means even symchoices without unresolved arguments aren't instantiated, which may be too restrictive, but it could also be too lenient and we might need to make symchoice instantiations always untyped. The behavior for symchoice is not sound anyway given it causes #9997 so this is something to consider for a redesign. Diff follows #24276. (cherry picked from commit 67ad1ae)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refs #8064, refs #24010
Error messages for standalone explicit generic instantiations are revamped. Failing standalone explicit generic instantiations now only error after overloading has finished and resolved to the default
[]
magic (this means[]
can now be overloaded for procs but this isn't necessarily intentional, in #24010 it was documented that it isn't possible). The error messages for failed instantiations are also no longer a simplecannot instantiate: foo
message, instead they now give the same type mismatch error message as overloads with mismatching explicit generic parameters.This is now possible due to the changes in #24010 that delegate all explicit generic proc instantiations to overload resolution. Old code that worked around this is now removed.
maybeInstantiateGeneric
could maybe also be removed in favor of justexplicitGenericSym
, theresult == n
case is due toexplicitGenericInstError
which is only for niche cases.Also, to cause "ambiguous identifier" error messages when the explicit instantiation is a symchoice and the expression context doesn't allow symchoices, we semcheck the sym/symchoice created by
explicitGenericSym
with the given expression flags.#8064 isn't entirely fixed because the error message is still misleading for the original case which does
values[1]
, as a consequence of #12664.